Skip to content

Improved Grade Distributions#1169

Open
fiveminus1 wants to merge 37 commits into
mainfrom
improved-grade-dist
Open

Improved Grade Distributions#1169
fiveminus1 wants to merge 37 commits into
mainfrom
improved-grade-dist

Conversation

@fiveminus1

@fiveminus1 fiveminus1 commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

Implements the improved grade distributions views from #1121. This includes new cards for average GPA, quality, and difficulty, but does not include the instructors view. The pie chart is now removed, and we're also replacing nivo charts with recharts, since nivo seems to be poorly maintained.

RFC on mobile display, loading skeletons, and light mode styling.

Screenshots

Old New
image image

Test Plan

  • Accurate data
  • Selections work
  • Styling's good

Issues

Closes #1121

@fiveminus1 fiveminus1 requested a review from CadenLee2 June 3, 2026 06:47

@CadenLee2 CadenLee2 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder of changes discussed in our meeting. Hopefully these are each pretty small:

  1. Change "Winter 2026" to "W26" for space to prevent the layout shift.

  2. Change the comparison direction and make it a delta rather than absolute, so that in Winter we can see "red down 0.2 from Fall" which means Winter's GPA was 0.2 less than Fall's

    • Image
  3. Possibly change the background color in light theme. #1149 will add a better overlay3 color once it's merged, so perhaps we can go with that.

  4. Change all of these emojis to the corresponding icons. We might as well change this on the professor preview page too; that allows us to remove the react-twemoji dependency! If you'd like, we can put this off for a different PR

    • Image
  5. Consider running an accessibility check (I believe we were talking about the chart color contrast here)

  6. Make the grade distribution chart + three info stats on the left a fixed height. This means that case 1) the down/up arrows are there, and case 2) the down/up arrows are not there, should have the same height.

UI/UX will work on a mobile redesign at some point (indeterminate), but in the meantime on small screens we should do a small fix to get

    1. instructor and quarter selectors either full width or on the same row, and
    1. these three statistics on the same row if possible. That might look awkward though so we could also consider putting them in a 2x2 grid with one slot empty, or having the GPA be full width above then the quality/difficutly in a row below it. Message me if this description is confusing lol.
Image

<ResponsiveContainer width="100%" height="100%">
<BarChart data={data}>
<XAxis dataKey="grade" />
<Tooltip

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat odd tooltip behavior: it always spawns in from the top left corner then moves toward your mouse, meaning if you quickly mouse out then back into the chart, it flickers/teleports.

  • If there's a way to easily fix it, let's go with that
  • Otherwise, probably just disable the animation so it always instantly follows the user's mouse

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be a known issue with custom content and I don't care enough for the animation, so we're disabling it

@anthonyj33 anthonyj33 mentioned this pull request Jun 16, 2026
@fiveminus1

Copy link
Copy Markdown
Contributor Author

@CadenLee2

Possibly change the background color in light theme. #1149 will add a better overlay3 color once it's merged, so perhaps we can go with that.

Used overlay2 to match reviewcards. Let me know what you think

Change all of these emojis to the corresponding icons. We might as well change this on the professor preview page too; that allows us to remove the react-twemoji dependency! If you'd like, we can put this off for a different PR

Let's do this in another PR since it'll remove a dependency and span throughout some other components

Consider running an accessibility check (I believe we were talking about the chart color contrast here)

I don't remember if we said we wanted the colors to be the same throughout the modes, but I changed up the colors to something that should be more WCAG-compliant on the contrast for both light/dark overlay2 card backgrounds. If we can separate it into light/dark again maybe we can brainstorm on the colors a little more, but if you like the current colors then we can keep them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve grade distribution

2 participants